r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

Does this actually work for blessing? Can we hook up x.py to run this?

I don't know. I don't have time to look into it right now.

Ok, I updated this to implement cargo test --bless src/tools/clippy.

@Mark-Simulacrum do you know when you'll have a chance to look at this? In the meantime I've been cherry-picking the commit for #84039.

@Manishearth can you either investigate or ping someone who could why we're seeing slightly different test outputs when blessing with this PR (Vec::new -> .., for example; see PR description).

cc @camsteffen @flip1995

unsure what's going on here, that looks like span_from_snippet failing but idk why

Also it could be divergences in compiletest. We tried to convince the compiler team to move compiletest out of tree but they weren't really in favor.

AFAIK the compiletest Clippy uses doesn't have bless support, so we implemented in Clippy on our own. @xFrednet and @phansch worked on this command mainly, so they probably have insight into this. Also why cargo dev bless changes files that shouldn't be changed.

can you either investigate or ping someone who could why we're seeing slightly different test outputs when blessing with this PR (Vec::new -> .., for example; see PR description).

It's also possible that I just need to rebase over master or something - I haven't spent too much time looking into this.

That sounds like it's not unlikely 😄

I'm sadly not totally up to date when it comes to the bless command due to some RL obligations. I believe @camsteffen has also worked on the bless command in Clippy, so he might know something about this 🙃

@jyn514 I checked out your branch, rebased it on top of the current master and tried ./x.py test --bless src/tools/clippy after modifying some stderr files in the clippy dir.It didn't update any test files other than the modified.

It would be great if you could also apply this patch:

diff --git a/src/tools/clippy/clippy_dev/src/new_lint.rs b/src/tools/clippy/clippy_dev/src/new_lint.rs
index d951ca0e630..4676c2affad 100644
--- a/src/tools/clippy/clippy_dev/src/new_lint.rs
+++ b/src/tools/clippy/clippy_dev/src/new_lint.rs
@@ -44,7 +44,7 @@ pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str
     create_test(&lint).context("Unable to create a test for the new lint")
 }
 
-fn create_lint(lint: &LintData) -> io::Result<()> {
+fn create_lint(lint: &LintData<'_>) -> io::Result<()> {
     let (pass_type, pass_lifetimes, pass_import, context_import) = match lint.pass {
         "early" => ("EarlyLintPass", "", "use rustc_ast::ast::*;", "EarlyContext"),
         "late" => ("LateLintPass", "<'_>", "use rustc_hir::*;", "LateContext"),
@@ -68,7 +68,7 @@ fn create_lint(lint: &LintData) -> io::Result<()> {
     write_file(lint.project_root.join(&lint_path), lint_contents.as_bytes())
 }
 
-fn create_test(lint: &LintData) -> io::Result<()> {
+fn create_test(lint: &LintData<'_>) -> io::Result<()> {
     fn create_project_layout<P: Into<PathBuf>>(lint_name: &str, location: P, case: &str, hint: &str) -> io::Result<()> {
         let mut path = location.into().join(case);
         fs::create_dir(&path)?;

It would be great if you could also apply this patch:

Sure thing, done.

So no outstanding concerns here? Just checking since I was tagged.

@jyn514 Can you update the PR description to reflect what I think is the current state, i.e., that this just works?

If so, r=me

Oops, I updated the commit message but not the description. Now fixed.

@bors r=Mark-Simulacrum

📌 Commit 8c25e27 has been approved by Mark-Simulacrum

⌛ Testing commit 8c25e27 with merge 267ddd8719e80bbcdf8613881b8f87788de74412...

💔 Test failed - checks-actions

A job failed! Check out the build log: (web) (plain)

 19  424M   19 84.6M    0     0  11.2M      0  0:00:37  0:00:07  0:00:30 11.1M
 21  424M   21 90.5M    0     0  10.5M      0  0:00:40  0:00:08  0:00:32 10.0M
 22  424M   22 96.5M    0     0  10.1M      0  0:00:41  0:00:09  0:00:32 9194k
 24  424M   24  101M    0     0  10.2M      0  0:00:41  0:00:09  0:00:32 8766k
curl: (56) OpenSSL SSL_read: Connection reset by peer, errno 54
clang+llvm-10.0.0-x86_64-apple-darwin/bin/modularize: Lzma library error:  No progress is possible
tar: Error exit delayed from previous errors.
##[error]Process completed with exit code 1.
[command]/usr/local/bin/git version
git version 2.31.1
[command]/usr/local/bin/git config --local --name-only --get-regexp core\.sshCommand
[command]/usr/local/bin/git submodule foreach --recursive git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :

curl: (56) OpenSSL SSL_read: Connection reset by peer, errno 54

@bors retry

⌛ Testing commit 8c25e27 with merge 10a51c0...

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 10a51c0 to master...

I'm making some changes to the structured suggestion output, and it seems like I'm unable to get my local environment to fail the tests that are failing in CI. I'm not sure if it is because cargo uitest is not actually using my locally built rustc/cargo (maybe due to the toolchain file?). I've tried invoking with ./x.py test src/tools/clippy --stage 2 --bless, which seems like it should do the right thing, but I am currently stuck. Could someone point me in the direction on how to keep clippy's tests in sync with rustc?

That should work. If it's not, I don't know what's going wrong, sorry.

I just lost two hours to being in the wrong branch 🤦‍♂️

Edit: BTW, thank you @jyn514 for improving the workflow here! Had it not been for my PEBKAC problem, the current behavior is almost perfect.